From dafab2408a3e9e0bb55655df0447dd1577969b72 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 10 Aug 2017 00:54:09 +0300 Subject: [PATCH] Allow to skip test code during publishing --- src/cargo/core/manifest.rs | 19 +++++++++++-- src/cargo/ops/cargo_compile.rs | 12 ++++++-- src/cargo/util/toml/mod.rs | 9 ++++-- src/cargo/util/toml/targets.rs | 50 +++++++++++++++++++++++----------- tests/build.rs | 32 ++++++++++++++++++++++ tests/test.rs | 39 ++++++++++++++++++++++++++ 6 files changed, 138 insertions(+), 23 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 75c09c5e2..193a3e0a8 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -22,7 +22,7 @@ pub struct Manifest { summary: Summary, targets: Vec, links: Option, - warnings: Vec, + warnings: Vec, exclude: Vec, include: Vec, metadata: ManifestMetadata, @@ -34,6 +34,15 @@ pub struct Manifest { original: Rc, } +/// When parsing `Cargo.toml`, some warnings should silenced +/// if the manifest comes from a dependency. `ManifestWarning` +/// allows this delayed emission of warnings. +#[derive(Clone, Debug)] +pub struct DelayedWarning { + pub message: String, + pub is_critical: bool +} + #[derive(Clone, Debug)] pub struct VirtualManifest { replace: Vec<(PackageIdSpec, Dependency)>, @@ -257,7 +266,7 @@ impl Manifest { pub fn summary(&self) -> &Summary { &self.summary } pub fn targets(&self) -> &[Target] { &self.targets } pub fn version(&self) -> &Version { self.package_id().version() } - pub fn warnings(&self) -> &[String] { &self.warnings } + pub fn warnings(&self) -> &[DelayedWarning] { &self.warnings } pub fn profiles(&self) -> &Profiles { &self.profiles } pub fn publish(&self) -> bool { self.publish } pub fn replace(&self) -> &[(PackageIdSpec, Dependency)] { &self.replace } @@ -272,7 +281,11 @@ impl Manifest { } pub fn add_warning(&mut self, s: String) { - self.warnings.push(s) + self.warnings.push(DelayedWarning { message: s, is_critical: false }) + } + + pub fn add_critical_warning(&mut self, s: String) { + self.warnings.push(DelayedWarning { message: s, is_critical: true }) } pub fn set_summary(&mut self, summary: Summary) { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 6d2ba3bd7..e4162b81f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -33,6 +33,7 @@ use core::resolver::Resolve; use ops::{self, BuildOutput, Executor, DefaultExecutor}; use util::config::Config; use util::{CargoResult, profile}; +use util::errors::{CargoResultExt, CargoError}; /// Contains information about how a package should be compiled. #[derive(Debug)] @@ -181,8 +182,15 @@ pub fn compile_with_exec<'a>(ws: &Workspace<'a>, exec: Arc) -> CargoResult> { for member in ws.members() { - for key in member.manifest().warnings().iter() { - options.config.shell().warn(key)? + for warning in member.manifest().warnings().iter() { + if warning.is_critical { + let err: CargoResult<_> = Err(CargoError::from(warning.message.to_owned())); + return err.chain_err(|| { + format!("failed to parse manifest at `{}`", member.manifest_path().display()) + }) + } else { + options.config.shell().warn(&warning.message)? + } } } compile_ws(ws, None, options, exec) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index fcf5a98d1..d20f262a7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -504,6 +504,7 @@ impl TomlManifest { -> CargoResult<(Manifest, Vec)> { let mut nested_paths = vec![]; let mut warnings = vec![]; + let mut errors = vec![]; let project = me.project.as_ref().or_else(|| me.package.as_ref()); let project = project.ok_or_else(|| { @@ -520,7 +521,8 @@ impl TomlManifest { // If we have no lib at all, use the inferred lib if available // If we have a lib with a path, we're done // If we have a lib with no path, use the inferred lib or_else package name - let targets = targets(me, package_name, package_root, &project.build, &mut warnings)?; + let targets = targets(me, package_name, package_root, &project.build, + &mut warnings, &mut errors)?; if targets.is_empty() { debug!("manifest has no build targets"); @@ -659,7 +661,10 @@ impl TomlManifest { `license-file` is necessary".to_string()); } for warning in warnings { - manifest.add_warning(warning.clone()); + manifest.add_warning(warning); + } + for error in errors { + manifest.add_critical_warning(error); } Ok((manifest, nested_paths)) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index ac35f59d5..70b84f815 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -24,7 +24,8 @@ pub fn targets(manifest: &TomlManifest, package_name: &str, package_root: &Path, custom_build: &Option, - warnings: &mut Vec) + warnings: &mut Vec, + errors: &mut Vec) -> CargoResult> { let mut targets = Vec::new(); @@ -42,15 +43,15 @@ pub fn targets(manifest: &TomlManifest, ); targets.extend( - clean_examples(manifest.example.as_ref(), package_root)? + clean_examples(manifest.example.as_ref(), package_root, errors)? ); targets.extend( - clean_tests(manifest.test.as_ref(), package_root)? + clean_tests(manifest.test.as_ref(), package_root, errors)? ); targets.extend( - clean_benches(manifest.bench.as_ref(), package_root, warnings)? + clean_benches(manifest.bench.as_ref(), package_root, warnings, errors)? ); // processing the custom build script @@ -180,7 +181,11 @@ fn clean_bins(toml_bins: Option<&Vec>, } else { None } - })?; + }); + let path = match path { + Ok(path) => path, + Err(e) => bail!("{}", e), + }; let mut target = Target::bin_target(&bin.name(), path, bin.required_features.clone()); @@ -210,11 +215,12 @@ fn clean_bins(toml_bins: Option<&Vec>, } fn clean_examples(toml_examples: Option<&Vec>, - package_root: &Path) + package_root: &Path, + errors: &mut Vec) -> CargoResult> { let targets = clean_targets("example", "example", toml_examples, inferred_examples(package_root), - package_root)?; + package_root, errors)?; let mut result = Vec::new(); for (path, toml) in targets { @@ -233,10 +239,11 @@ fn clean_examples(toml_examples: Option<&Vec>, } fn clean_tests(toml_tests: Option<&Vec>, - package_root: &Path) -> CargoResult> { + package_root: &Path, + errors: &mut Vec) -> CargoResult> { let targets = clean_targets("test", "test", toml_tests, inferred_tests(package_root), - package_root)?; + package_root, errors)?; let mut result = Vec::new(); for (path, toml) in targets { @@ -250,7 +257,8 @@ fn clean_tests(toml_tests: Option<&Vec>, fn clean_benches(toml_benches: Option<&Vec>, package_root: &Path, - warnings: &mut Vec) -> CargoResult> { + warnings: &mut Vec, + errors: &mut Vec) -> CargoResult> { let mut legacy_bench_path = |bench: &TomlTarget| { let legacy_path = package_root.join("src").join("bench.rs"); if !(bench.name() == "bench" && legacy_path.exists()) { @@ -267,6 +275,7 @@ fn clean_benches(toml_benches: Option<&Vec>, let targets = clean_targets_with_legacy_path("benchmark", "bench", toml_benches, inferred_benches(package_root), package_root, + errors, &mut legacy_bench_path)?; let mut result = Vec::new(); @@ -283,12 +292,14 @@ fn clean_benches(toml_benches: Option<&Vec>, fn clean_targets(target_kind_human: &str, target_kind: &str, toml_targets: Option<&Vec>, inferred: Vec<(String, PathBuf)>, - package_root: &Path) + package_root: &Path, + errors: &mut Vec) -> CargoResult> { clean_targets_with_legacy_path(target_kind_human, target_kind, toml_targets, inferred, package_root, + errors, &mut |_| None) } @@ -296,6 +307,7 @@ fn clean_targets_with_legacy_path(target_kind_human: &str, target_kind: &str, toml_targets: Option<&Vec>, inferred: Vec<(String, PathBuf)>, package_root: &Path, + errors: &mut Vec, legacy_path: &mut FnMut(&TomlTarget) -> Option) -> CargoResult> { let toml_targets = match toml_targets { @@ -316,7 +328,14 @@ fn clean_targets_with_legacy_path(target_kind_human: &str, target_kind: &str, validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); for target in toml_targets { - let path = target_path(&target, &inferred, target_kind, package_root, legacy_path)?; + let path = target_path(&target, &inferred, target_kind, package_root, legacy_path); + let path = match path { + Ok(path) => path, + Err(e) => { + errors.push(e); + continue + }, + }; result.push((path, target)); } Ok(result) @@ -443,7 +462,7 @@ fn target_path(target: &TomlTarget, inferred: &[(String, PathBuf)], target_kind: &str, package_root: &Path, - legacy_path: &mut FnMut(&TomlTarget) -> Option) -> CargoResult { + legacy_path: &mut FnMut(&TomlTarget) -> Option) -> Result { if let Some(ref path) = target.path { // Should we verify that this path exists here? return Ok(package_root.join(&path.0)); @@ -462,9 +481,8 @@ fn target_path(target: &TomlTarget, if let Some(path) = legacy_path(target) { return Ok(path); } - - bail!("can't find `{name}` {target_kind}, specify {target_kind}.path", - name = name, target_kind = target_kind) + Err(format!("can't find `{name}` {target_kind}, specify {target_kind}.path", + name = name, target_kind = target_kind)) } (None, Some(_)) => unreachable!() } diff --git a/tests/build.rs b/tests/build.rs index a673c1fe7..0aacd6cf1 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -3549,3 +3549,35 @@ fn same_metadata_different_directory() { ), ); } + +#[test] +fn building_a_dependent_crate_witout_bin_should_fail() { + Package::new("testless", "0.1.0") + .file("Cargo.toml", r#" + [project] + name = "testless" + version = "0.1.0" + + [[bin]] + name = "a_bin" + "#) + .file("src/lib.rs", "") + .publish(); + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + + [dependencies] + testless = "0.1.0" + "#) + .file("src/lib.rs", ""); + p.build(); + + assert_that(p.cargo("build"), + execs().with_status(101).with_stderr_contains( + "[..]can't find `a_bin` bin, specify bin.path" + )); +} diff --git a/tests/test.rs b/tests/test.rs index d0d577d09..b4dc3b0c6 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -2732,3 +2732,42 @@ fn cyclic_dev() { assert_that(p.cargo_process("test").arg("--all"), execs().with_status(0)); } + +#[test] +fn publish_a_crate_without_tests() { + Package::new("testless", "0.1.0") + .file("Cargo.toml", r#" + [project] + name = "testless" + version = "0.1.0" + exclude = ["tests/*"] + + [[test]] + name = "a_test" + "#) + .file("src/lib.rs", "") + + // In real life, the package will have a test, + // which would be excluded from .crate file by the + // `exclude` field. Our test harness does not honor + // exclude though, so let's just not add the file! + // .file("tests/a_test.rs", "") + + .publish(); + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + + [dependencies] + testless = "0.1.0" + "#) + .file("src/lib.rs", ""); + p.build(); + + assert_that(p.cargo("test"), execs().with_status(0)); + assert_that(p.cargo("test").arg("--package").arg("testless"), + execs().with_status(0)); +} -- 2.30.2